-
Notifications
You must be signed in to change notification settings - Fork 54
Handle non-fully-active documents (and destroyed execution contexts) #597
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive guidance for handling cases where documents become non-fully-active or execution contexts are destroyed, such as when iframes are removed or navigated away. The update provides clear specifications for managing the lifecycle and cleanup of asynchronous operations and UI elements in these scenarios.
- Defines behavior for API calls when the associated document is not fully active at call time
- Specifies handling of ongoing async operations when contexts become destroyed or non-fully-active
- Establishes guidance for queued tasks and cleanup semantics without relying on garbage collection
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
index.bs
Outdated
to protect users from invasive behaviours, | ||
and seeking [meaningful consent](#consent) is also important. | ||
|
||
<h3 id="handle-destroyed-contexts">Handle non-fully-active documents (and destroyed execution contexts)</h3> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everywhere else we use "non-fully active" ... which is kinda wrong, but allows [=fully active=] to work. We should be consistent about the hyphenation across this whole document.
See also: [[#support-non-fully-active]] for the BFCache case; | ||
most of the same principles apply to detached iframes | ||
and other torn-down [=All Execution Contexts/realms=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like these sections need to be integrated. Is there a set of instructions that works for all non-fully-active documents, with small extensions that could live in subsections for the bfcache and iframe cases?
When an <{iframe}> (or similar) is removed or navigated, | ||
its document stops being [=Document/fully active=] | ||
and its [=ECMAScript/execution contexts=] can get destroyed. | ||
However, references to objects from that document | ||
can remain reachable from other documents | ||
or from other realms. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As in other PRs, sections should start with the main, positive, advice. Maybe:
Avoid doing work, and usually cancel ongoing work, on non-[=fully active=] documents and destroyed execution contexts.
(With a link to the specification terms that should be used to detect a destroyed execution context.)
that [=associated Document=] becomes non-[=Document/fully active=] | ||
or the [=relevant global object=]'s [=ECMAScript/execution context=] is destroyed by script, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should link to the algorithm to patch for every asynchronous operation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might say "As a general rule, ..." and maybe mention that special case.
The intent here is that if the developer destroys the execution context (e..g, iframe.remove()
), they generally intended (or should be interpreted by the UA) as them wanting to abort all ongoing operations.
The case you are talking about is different: when it's a user initiated action that forces a document into the BFCache, and hence the document becomes non-fully active. In that case, it makes sense to continue/resume loading things like images or other media (i.e., user needs VS developer wants).
If, after starting an asynchronous operation, | ||
that [=associated Document=] becomes non-[=Document/fully active=] | ||
or the [=relevant global object=]'s [=ECMAScript/execution context=] is destroyed by script, | ||
abort the operation and reject with {{AbortError}}. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to always be the right advice. E.g. https://html.spec.whatwg.org/multipage/images.html#update-the-image-data just waits for the document to become fully active again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but this is a special case... document cannot be made non-fully active by script in way that is recoverable to make them fully active again (i.e., AFAIK, only user action can put a document into a recoverable state, but never via script).
When [=queue a task|queueing=] subsequent work | ||
(e.g., for an event, settling a promise, a time-based operation), | ||
include an early exit: | ||
if the [=associated Document=] is not [=Document/fully active=], | ||
gracefully terminate the [=task=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This paragraph seems to be saying how to accomplish the previous paragraph. Do folks struggle enough with figuring out how to abort an operation that we need to spend words on it? Is this even always the right strategy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My experience is that yes, and also because there are the tricky edge cases, even implementers might have not thought of these cases. These are where you get weird situations around unspecified behavior.
|
||
When an <{iframe}> (or similar) is removed or navigated, | ||
its document stops being [=Document/fully active=] | ||
and its [=ECMAScript/execution contexts=] can get destroyed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a "destroyed" execution context a concept that can appear in specifications? If not, we should only mention the spec-visible conditions that cause this to happen in implementations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's basically:
- Removing an iframe.
- Navigating an iframe.
Both destroy the script execution context, causing all the destructors to run... and some of those would resolve pending promises with AbortError, and tear down UIs (e.g., payment sheet, a credential chooser, and so on).
This pull request adds new guidance for handling cases where documents or execution contexts are no longer fully active, such as when an iframe is removed or navigated away. The update clarifies how specifications should address the lifecycle and cleanup of asynchronous operations and UI elements associated with destroyed or inactive contexts.
Guidance for handling destroyed contexts and non-fully-active documents:
#handle-destroyed-contexts
) recommending that specs explicitly define behavior when the associated document is not fully active at call time (e.g., throw or reject withInvalidStateError
).AbortError
if the context becomes non-fully-active or the execution context is destroyed, and that any UI should be dismissed in such cases.Preview | Diff